Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SYCL] [libdevice] Add vector overloads of ConvertBFloat16ToFINTEL and ConvertFToBFloat16INTEL #14085

Merged
merged 11 commits into from
Jun 12, 2024

Conversation

uditagarwal97
Copy link
Contributor

@uditagarwal97 uditagarwal97 commented Jun 6, 2024

This PR adds vector overloads of ConvertBFloat16ToFINTEL and ConvertFToBFloat16INTEL to libdevice (SPEC: https://spec.oneapi.io/level-zero/latest/core/SPIRV.html#bfloat16-conversions) and a wrapper around it (BF16VecToFloatVec and FloatVecToBF16Vec) in ext::oneapi::detail.

These overloads are intended to optimize BFloat16 marray, vec operations, for which we currently do element-by-element bfloat16 -> float -> bfloat16 conversions.

@uditagarwal97 uditagarwal97 marked this pull request as ready for review June 7, 2024 01:01
@uditagarwal97 uditagarwal97 requested review from a team as code owners June 7, 2024 01:01
@uditagarwal97
Copy link
Contributor Author

@intel/dpcpp-tools-reviewers friendly ping!

Copy link
Contributor

@asudarsa asudarsa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sycl-post-link changes are trivial and look good. Thanks.

}).wait();

if (err)
std::cout << "failed\n";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if there is a future change in the source that causes this test to fail someday, it'll be more work for whoever catches that ticket. Make the failure output more explicit. At minimum output "failed on host" and "failed on device". And if you know of anything else that might be useful to some unlucky co-worker, maybe include that too.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure. I'll improve the test output.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, after looking at the test again, the test will currently output something like:

    bfloat16[4] -> float[4] conversion on device...  passed/failed

or

float[4] -> bfloat16[4] -> float[4] conversion on host.... passed/failed

for device and host respectively. There's a std::cout before queue.submit() which will output the type of conversion being performed along where the conversion takes place (host/device).

Copy link
Contributor

@cperkinsintel cperkinsintel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lime Gin Tastes Metallic.

just one request for better test output

@aelovikov-intel aelovikov-intel merged commit 13a7b3a into intel:sycl Jun 12, 2024
14 checks passed
}).wait();

if (err)
std::cout << "failed\n";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this go to std::cerr instead of std::cout?

ianayl pushed a commit to ianayl/sycl that referenced this pull request Jun 13, 2024
…d ConvertFToBFloat16INTEL (intel#14085)

This PR adds vector overloads of `ConvertBFloat16ToFINTEL` and
`ConvertFToBFloat16INTEL` to libdevice (SPEC:
https://spec.oneapi.io/level-zero/latest/core/SPIRV.html#bfloat16-conversions)
and a wrapper around it (`BF16VecToFloatVec` and `FloatVecToBF16Vec`) in
`ext::oneapi::detail`.

These overloads are intended to optimize BFloat16 `marray`, `vec`
operations, for which we currently do element-by-element `bfloat16 ->
float -> bfloat16` conversions.
againull pushed a commit that referenced this pull request Jun 21, 2024
…cl::vec` of `bfloat16` type to that of other data types (#14105)

Follow-up of and blocked by: #14085

After this change:
On host, conversion between `vec<bfloat16>` and `vec<float>` will happen
element-by-element. While on device, we'll use Spirv intrinsic
`OpConvertFToBF16INTEL` and `OpConvertBF16ToFINTEL`
(https://github.com/intel/llvm/blob/sycl/sycl/doc/design/spirv-extensions/SPV_INTEL_bfloat16_conversion.asciidoc)
for vector conversion.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants